Terraform provider ProfitBricks - Data Sources #11520

jasmingacic · 2017-01-30T17:43:01Z

While this PR is fresh I have two issues with unit tests:

resource_profitbricks_loadbalancer_test - The issue is within ProfitBricks API. When a load balancer is provisioned it creates a private LAN and attaches it to existing NIC. This is leaving .tfstate with "dangling resources". This is a known issue with ProfitBricks
resource_profitbricks_volume_test - The script used for unit tests is working when ran separately but once unit tests run I get this error:

        testing.go:265: Step 1 error: Check failed: Check 1/2 error: Bad name: webserver

I'm not sure how to fix this.
@stack72 do you have any ideas?

…r-profitbricks

…r-profitbricks # Conflicts: # command/internal_plugin_list.go

…r-profitbricks

- nat parameter for Nics - availabilityZone for Volumes Minor code clean up

jasmingacic · 2017-01-31T16:45:52Z

I'm using this PR to update to ProfitBicks REST V3. Adding following parameters:

nat parameter for Nics
availabilityZone for Volumes

stack72 · 2017-02-01T14:47:33Z

👀 are on this now @jasminSPC

stack72

Hey @jasminSPC

So the new data sources look good - I am not quite sure what we can do about the dangling resources

If this is an error on the profitbricks side, then we may need to have a step n the CheckDestroy that cleans everything up for us. Alternatively, we can have a script that runs after the tests that cleans up any dangling resources - the maintainer of the scaleway provider did the same thing here

With regards to that unit test - let's not worry too much about that right now - I will dig into that soon and have a look at what is going on there

P.

stack72 · 2017-02-01T14:51:56Z

builtin/providers/profitbricks/data_source_datacenter.go

+		return fmt.Errorf("An error occured while fetching datacenters %s", datacenters.Response)
+	}
+
+	name, _ := d.GetOk("name")


as it's a required field we can guarantee it's there so we can write

name := d.Get("name").(string)

stack72 · 2017-02-01T14:52:37Z

builtin/providers/profitbricks/data_source_datacenter_test.go

+	"testing"
+)
+
+func TestAccDataSourceDatacenter_basic(t *testing.T) {


I'd like to see a test that matches a datacenter name here :)

This one is tricky in order to get a match we need to have a data center. I'm not sure how to accomplish that?

In the test, have 2 steps, the first as a setup to create the datacenter, then the second will include a datacenter and a lookup

have a look at this test as an example https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/data_source_aws_autoscaling_groups_test.go#L15

I tried something similar:

In first step I do this:

resource "profitbricks_datacenter" "foobar" { name = "datacenter-test" location = "us/las" }

and then this.

resource "profitbricks_datacenter" "foobar" { name = "datacenter-test-updated" location = "us/las" } data "profitbricks_datacenter" "data_source"{ name = "datacenter-test-updated" location= "us/las" }

Each time I do this it simply doesn't work.
How do you want me to proceed?

stack72 · 2017-02-01T14:53:50Z

builtin/providers/profitbricks/data_source_image.go

+		return fmt.Errorf("An error occured while fetching ProfitBricks locations %s", images.Response)
+	}
+
+	name, _ := d.GetOk("name")


Name isn't required, so we should check it's ok before using it

stack72 · 2017-02-01T14:54:35Z

builtin/providers/profitbricks/data_source_datacenter.go

+				Type:     schema.TypeString,
+				Optional: true,
+			},
+			"id": {


We actually don't need to expose Id as a param - it's there by default

stack72 · 2017-02-01T14:54:40Z

builtin/providers/profitbricks/data_source_image.go

+				Type:     schema.TypeString,
+				Optional: true,
+			},
+			"id": {


We actually don't need to expose Id as a param - it's there by default

stack72 · 2017-02-01T14:55:09Z

builtin/providers/profitbricks/data_source_image.go

+		return fmt.Errorf("There are no images that match the search criteria")
+	}
+
+	d.Set("name", results[0].Properties.Name)


should we check that Properties != nil ? Otherwise we could panic here

Properties ImageProperties `json:"properties,omitempty"
It can't be nil it is not a pointer.

👍 sorry - still in AWS land where everything is a pointer :)

stack72 · 2017-02-01T14:55:22Z

builtin/providers/profitbricks/data_source_location.go

+				Type:     schema.TypeString,
+				Optional: true,
+			},
+			"id": {


We actually don't need to expose Id as a param - it's there by default

jasmingacic · 2017-02-01T21:16:22Z

The loadbalancer thing is a known issue with ProfitBricks and we can't do much about that.
I can add cleanup in check destroy to wipe all orphaned resource.

…ng sure that the test doesn't leave dangling resources in ProfitBricks

stack72 · 2017-02-02T10:42:47Z

Hey

so i just went through this and it looks good - I ran the tests and got 1 error

% make testacc TEST=./builtin/providers/profitbricks TESTARGS='-run=TestAccDataSource'                                                   1 ↵ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/02 10:39:44 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/profitbricks -v -run=TestAccDataSource -timeout 120m
=== RUN   TestAccDataSourceDatacenter_basic
--- PASS: TestAccDataSourceDatacenter_basic (0.64s)
=== RUN   TestAccDataSourceImage_basic
--- FAIL: TestAccDataSourceImage_basic (0.55s)
	testing.go:265: Step 0 error: Check failed: Check 2/3 error: data.profitbricks_image.img: Attribute 'name' expected "Ubuntu-16.04-LTS-server-2017-01-01", got "Ubuntu-16.04-LTS-server-2017-02-01"
=== RUN   TestAccDataSourceLocation_basic
--- PASS: TestAccDataSourceLocation_basic (0.61s)
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/profitbricks	1.824s
make: *** [testacc] Error 1

I think we are going to have an issue with that test if it revs a version each month though so all in all this is looking good

P.

jasmingacic · 2017-02-02T11:05:37Z

The problem with the test is that ProfitBricks is changing image names on monthly basis.
I guess I should add custom check for this.
@stack72 What do you think?

Code clean up

stack72 · 2017-02-02T13:26:07Z

This LGTM! :)

% make testacc TEST=./builtin/providers/profitbricks TESTARGS='-run=TestAccDataSource'                                                       ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/02 13:25:28 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/profitbricks -v -run=TestAccDataSource -timeout 120m
=== RUN   TestAccDataSourceDatacenter_matching
--- PASS: TestAccDataSourceDatacenter_matching (12.29s)
=== RUN   TestAccDataSourceImage_basic
--- PASS: TestAccDataSourceImage_basic (0.67s)
=== RUN   TestAccDataSourceLocation_basic
--- PASS: TestAccDataSourceLocation_basic (0.46s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/profitbricks	13.433s

ghost · 2020-04-17T02:07:46Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

jasminSPC added 16 commits August 3, 2016 17:47

Terraform ProfitBricks Builder

fa0b8fc

Merge remote-tracking branch 'upstream/master' into terraform-provide…

3b907d6

…r-profitbricks

make fmt

940b3f6

Merge remote-tracking branch 'upstream/master' into terraform-provide…

5d63bd9

…r-profitbricks

Merge remote-tracking branch 'upstream/master' into terraform-provide…

9c82c60

…r-profitbricks # Conflicts: # command/internal_plugin_list.go

Merge remote-tracking branch 'upstream/master' into terraform-provide…

5996086

…r-profitbricks

Merge remote-tracking branch 'upstream/master' into terraform-provide…

ac2f78f

…r-profitbricks

Merge remote-tracking branch 'upstream/master' into terraform-provide…

5a6dd69

…r-profitbricks

Addressing PR remarks

379a6cc

Merge remote-tracking branch 'upstream/master' into terraform-provide…

afabb26

…r-profitbricks

Merge remote-tracking branch 'upstream/master' into terraform-provide…

21831c0

…r-profitbricks

Removed importers

cff8e3f

Merge remote-tracking branch 'upstream/master' into terraform-provide…

62c0b04

…r-profitbricks

Merge remote-tracking branch 'upstream/master' into terraform-provide…

501e606

…r-profitbricks

Added ProfitBricks Data Sources

e1fc5e0

Added documentation

2f6a702

jasmingacic added the new-data-source label Jan 30, 2017

jasminSPC added 3 commits January 31, 2017 17:01

Merge remote-tracking branch 'upstream/master' into terraform-provide…

db4afee

…r-profitbricks

Updated to REST v3:

41b73d6

- nat parameter for Nics - availabilityZone for Volumes Minor code clean up

Minor code clean up

68e7ddb

jasminSPC added 2 commits January 31, 2017 17:49

Fixed typo in volume documentation

2e3618d

make fmt

9107784

stack72 reviewed Feb 1, 2017

View reviewed changes

stack72 added enhancement provider/profitbricks labels Feb 1, 2017

Addressing requested changes

c0910a4

Added a step in load balancer tests in CheckDestroy where we are maki…

80c650e

…ng sure that the test doesn't leave dangling resources in ProfitBricks

jasminSPC added 2 commits February 2, 2017 12:18

Changed expected image name

1cf5f09

Fixed data center test

913f0d1

Code clean up

stack72 merged commit b862cd2 into hashicorp:master Feb 2, 2017

ghost locked and limited conversation to collaborators Apr 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Terraform provider ProfitBricks - Data Sources #11520

Terraform provider ProfitBricks - Data Sources #11520

jasmingacic commented Jan 30, 2017

jasmingacic commented Jan 31, 2017

stack72 commented Feb 1, 2017

stack72 left a comment

stack72 Feb 1, 2017

stack72 Feb 1, 2017

jasmingacic Feb 1, 2017 •

edited

Loading

stack72 Feb 1, 2017

jasmingacic Feb 1, 2017 •

edited

Loading

stack72 Feb 1, 2017

stack72 Feb 1, 2017

stack72 Feb 1, 2017

stack72 Feb 1, 2017

jasmingacic Feb 1, 2017

stack72 Feb 1, 2017

stack72 Feb 1, 2017

jasmingacic commented Feb 1, 2017

stack72 commented Feb 2, 2017

jasmingacic commented Feb 2, 2017

stack72 commented Feb 2, 2017

ghost commented Apr 17, 2020

Terraform provider ProfitBricks - Data Sources #11520

Terraform provider ProfitBricks - Data Sources #11520

Conversation

jasmingacic commented Jan 30, 2017

jasmingacic commented Jan 31, 2017

stack72 commented Feb 1, 2017

stack72 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasmingacic Feb 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasmingacic Feb 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasmingacic commented Feb 1, 2017

stack72 commented Feb 2, 2017

jasmingacic commented Feb 2, 2017

stack72 commented Feb 2, 2017

ghost commented Apr 17, 2020

jasmingacic Feb 1, 2017 •

edited

Loading

jasmingacic Feb 1, 2017 •

edited

Loading